-
Notifications
You must be signed in to change notification settings - Fork 302
feat(metrics): add price update delay metric and update method #2619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for a lot of change of minds here, i was wondering whether we can expose the raw data behind delay here and calculate the delay in our metrics platform instead. In this way we can expose the details of the delay in the dashboards as well.
apps/price_pusher/src/metrics.ts
Outdated
| public lastPublishedTime: Gauge<string>; | ||
| public priceUpdateAttempts: Counter<string>; | ||
| public priceFeedsTotal: Gauge<string>; | ||
| public priceUpdateDelay: Gauge<string>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, why are these string :?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are for the label names
…target timestamps
apps/price_pusher/src/metrics.ts
Outdated
| registers: [this.registry], | ||
| }); | ||
|
|
||
| this.targetTimestamp = new Gauge({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the difference between this and lastPublishedTime ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah its redundant, i will remove targetTimestamp and use lastPublishedTime instead
|
Ah please bump the version too. |
…lished time logic
Summary
Added a new gauge metric
pyth_price_update_delayto track the delay between source and target timestamps relative to each price feed's configured threshold. The metric is calculated as:Rationale
timeDifferenceconfiguration, making static thresholds suboptimalsourceLatestPricewon't update, keeping the delay constantdelay > 2min) while respecting individual feed configurationsHow has this been tested?
Steps to verify:
sourceLatestPrice.publishTime - targetLatestPrice.publishTimeagainst raw timestampspriceConfigTimeDifferenceThe changes are non-invasive and purely additive to the metrics system, with no impact on core price pushing functionality.